Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mouse pointer reappears at same spot #48980

Merged
merged 1 commit into from
May 23, 2021
Merged

Conversation

amoriqbal
Copy link
Contributor

@amoriqbal amoriqbal commented May 22, 2021

This fixes #48971

Before changes:
After grabbing and dragging a numeric field, mouse pointer is teleported to top-left.

After changes:
After grabbing and dragging a numeric field, mouse pointer reappears at the same point from where it vanished.
Edit: After incorporating the changes suggested by @EricEzaM, the mouse pointer should now reappear at the spot from where the user grabbed and started dragging.

Note: This PR does not affect the grab and drag behaviour of Spin Sliders. The problem does not exist with Spin Sliders.

@amoriqbal
Copy link
Contributor Author

Technical details:
godot/editor/editor_spin_slider.h declares grabbing_spinner_mouse_pos. Its default value is Vector2(0, 0).
When user grabs a numeric field (not a spinner) and drags, the mouse pointer is made invisible.
When the user stops dragging, the mouse pointer is made visible again at position grabbing_spinner_mouse_pos. But value of grabbing_spinner_mouse_pos is Vector2(0, 0). So mouse pointer re-appears at top left corner.

If we set grabbing_spinner_mouse_pos = get_global_mouse_position() before making the mouse pointer invisible, the mouse pointer reappears at the same point from where it vanished

Difference between behaviour of spin sliders and other numeric fields:
In spin sliders, the mouse pointer appears at the centre-bottom of the Rect. But for other numeric fields, the mouse pointer reappears from where it disappeared.

Imo, reappearing at original position is good as a default. Customisation for specific fields can be done thereafter.
Eg: grabbing_spinner_mouse_pos = get_global_position() + grabber_rect.position + grabber_rect.size * 0.5 is the customization for spin sliders. Removing this line of code, will make Spin Sliders follow the new default behaviour too.

@Calinou Calinou added this to the 4.0 milestone May 22, 2021
Copy link
Contributor

@EricEzaM EricEzaM left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your first time contribution! Some notes below

editor/editor_spin_slider.cpp Outdated Show resolved Hide resolved
@amoriqbal
Copy link
Contributor Author

Thanks for your first time contribution! Some notes below

I will make the necessary changes.

Copy link
Contributor

@EricEzaM EricEzaM left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks! I appreciate the time & effort you put into understanding the code, as shown by your comments above, despite this being a relatively small change. Hopefully the first of many contributions to come!

@akien-mga akien-mga merged commit ad72f3c into godotengine:master May 23, 2021
@akien-mga
Copy link
Member

Thanks! And congrats for your first merged Godot contribution 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants